build

Greptile Summary

This PR fixes GpuLike to match CPU semantics when a LIKE pattern contains an invalid escape sequence (e.g. LIKE 'm%@ca' ESCAPE '%'). Previously cuDF silently accepted such patterns and returned wrong results; now the GPU planner detects them at planning time and falls back to CPU, which throws the correct AnalysisException. The SPARK-33677 test exclusion (issue #14117) is removed as a result.

Key changes:

• Adds tagExprForGpu to BinaryExprMeta[Like] in GpuOverrides.scala that iterates the literal pattern, detects invalid escape sequences (escape char not followed by _, %, or the escape char itself, or trailing escape char), and calls willNotWorkOnGpu to trigger CPU fallback.
• Removes the SPARK-33677: LikeSimplification should be skipped if pattern contains any escapeChar exclusion from RapidsTestSettings.scala since the bug is now fixed.
• The PR description mentions adding defensive validation inside GpuLike.doColumnar as a safety net, but this was not included in the diff — stringFunctions.scala is unchanged. For the known code paths this does not expose a regression, but the PR description is inaccurate on this point.

Confidence Score: 4/5

• Safe to merge — the validation logic is correct and the fallback to CPU is the right behaviour for invalid escape patterns.
• The escape-validation logic in tagExprForGpu is correct: it walks the literal pattern, detects trailing escape chars and chars not in {_, %, escape} after an escape, and falls back to CPU. The parent's tagExprForGpu is empty so not calling super is fine and consistent with the rest of the codebase. The one deduction is that the PR description claims a doColumnar safety net was added but it was not; while this is not a runtime regression today (all invalid patterns are caught at plan time or rejected by ExprChecks), the description is misleading.
• No files require special attention beyond the noted discrepancy between the PR description and the absence of defensive validation in GpuLike.doColumnar.

Important Files Changed

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[LIKE expression reaches GPU planner] --> B{a.right is Literal UTF8String?}
    B -- No --> E[case _ : no validation, ExprChecks already rejects non-literals]
    B -- Yes --> C[Iterate pattern chars with tagExprForGpu]
    C --> D{Current char == escapeChar?}
    D -- No --> F[Advance i, continue]
    F --> D
    D -- Yes --> G{Next char exists?}
    G -- No trailing escape --> H[willNotWorkOnGpu → CPU fallback → AnalysisException]
    G -- Yes --> I{Next char in '_', '%', escapeChar?}
    I -- No invalid escape --> H
    I -- Yes valid escape --> J[Advance i by 2, continue]
    J --> D
    C --> K[All chars valid → convertToGpu → GpuLike.doColumnar → cuDF like]

_{Last reviewed commit: bd7f8eb}

build

build

Add defensive validation in GpuLike.doColumnar as a safety net.

Not needed. Because there will be no GpuLike if validation is failed.

Add the checklist in the desc of this PR:
### Checklists

- [ ] This PR has added documentation for new or modified features or
behaviors.
- [ ] This PR has added new tests or modified existing tests to cover
new code paths.
(Please explain in the PR description how the new code paths are tested,
such as names of the new/existing tests that cover them.)
- [ ] Performance testing has been performed and its results are added
in the PR description. Or, an issue has been filed with a link in the PR
description.

What is the performance impact of this change? All PRs are supposed to have a performance impact unless it is something like a doc or comment only change.

Update the PR description for performance impact with latest change according to review comments. Now only checks at planning stage once with a O(n) logic, where Like strings are usually not that super long.

@revans2 I'm trying to add rules to LLM that we should conduct performance regression check after touching any core functional codes. My question is, what kind of performance check are proper for these changes.

I can think of 2 types in general:

• NDS
• micro benchmarks

For NDS, I saw some colleagues do it when it comes to like shuffle optimization, join optimization, rules optimizations etc.

For micro benchmarks, I saw cuDF has a lot for it, also I recall some micro-benchmarks designed for like long string operations in spark-rapids.

Use this regex change for example, I prefer micro benchmark for it instead of NDS if it really needs a performance regression check. What's your suggestion on this top? A general rule to choose what type of performance checks to apply?

@wjxiz1992 we need to use different benchmarks for different reasons and situations. If the change targets a specific set of functionality ( an expression for example, or columnar to row modification), then a targeted micro benchmark can isolate the impact of the change. If the change is general, or the micro benchmark showed a significant regression then a more real world benchmark is helpful to either put the regression in context of real world processing or to exercise the broad change. But we have to be sure that that "real world" benchmark exercises the code we care about. NDS does not fall back to the CPU for any processing. That is good to show shuffle, but not columnar to row changes. NDS does not use maps, or arrays, or structs. So it is not good to test them. It hardly uses strings like real world queries do, so it is bad at that too. We often come up with other more realistic queries or use inspiration from customer queries we know of to help build some of these types of tests.

In general I want to think about how this change could impact the performance of the system and then build tests that would cover that case. Here we have escape checks that happen at planning time and escape checks that are happening at run time. The run time checks happen on a per-batch basis. So I probably want a few different tests. Some that are more malicious and some more realistic.

The "real world" test would be something like. spark.time(spark.range(0, 4294967296L, 1, 32).selectExpr("CAST(id as STRING) as sid").filter("SID LIKE '%B1_B1%' ESCAPE 'B'").count) I didn't check the syntax, but it should give about a 1 GiB batch to each task and give us 32 tasks to work with (assuming that we have a 1 GiB target batch size and we have 8 or 16 CPU cores to work with for spark running in local mode). This way we get parallelism on the GPU and multiple waves. I typically will capture a history file with this too and compare the optime for the operator that includes the change (probably filter in this case). For this case you have to have some knowledge of how Spark deal with the like operator. If the pattern is static then it may optimize some patterns into other operators. So we cannot do simple things, for example '%1%' is converted into a contains.

If I want to be malicious, then I start to change things around. We can set the target batch size much smaller (10 MiB for example so we get about 100x the number of batches) and see if the runtime check at the beginning of each batch has a measurable impact for before and after.

We could also do a lot of 2 or three row queries in a loop to see if the planning change has a measurable impact.

If we test what we think is the worst case situations and there is no measurable impact then we know it is not a big deal.

For this case I would not likely use NDS at all. There are few LIKE operations in it. There are few strings too, and the size of the string data isn't large, also most of the patterns that NDS uses are ones that Spark will rewrite to not use LIKE directly at all.